Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

quote slash (/) as a key separator when communicating with augeas #79

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

quote slash (/) as a key separator when communicating with augeas #79

wants to merge 2 commits into from

Conversation

johnwarburton
Copy link

Pull Request (PR) description

Quote sysctl keys with a slash in them

This Pull Request (PR) fixes the following issues

Partially fixes #17 by quoting slashes in keys (such as VLANs) when communicating with augeas

This requires augeas 1.14.x with patch to sysctl lens, that supports more non-alphanumeric characters such as a slash. Have tested by directly copying ne wlens to /opt/puppetlabs/puppet/share/augeas/lenses/dist/sysctl.aug of puppet agent 7.25

A new puppet agent with augeas 1.14.x needs to be advocated

Note that the changes provided here are probably not optimal and possibly break with comments, but its at the limit of my current provider knowledge

@johnwarburton johnwarburton mentioned this pull request Jul 12, 2023
Open
@johnwarburton
Copy link
Author

I am not sure what to do about the archlinux test failing

@bastelfreak
Copy link
Member

@johnwarburton ignore the archlinux job for now, we need to fix that in our CI. Are you able to add tests for this change?

@johnwarburton
Copy link
Author

johnwarburton commented Jul 14, 2023 •
edited
Loading

Hi @bastelfreak - sure

I am not sure what image you are using for CI, but the puppet-dev-tools image has augeas 1.11, and we need, at least 1.14 to get the new sysctl lens that accepts a slash in the key. Otherwise these tests fail trying to load the test data

How can we ensure we are testing with a 1.14 lens? I've force installed http://ftp.au.debian.org/debian/pool/main/a/augeas/augeas-lenses_1.14.0-1_all.deb to test. I am reluctant to submit the tests in this PR if they fail immediately due to the old lens being present

UPDATE - nope
augeas 1.11 with the 1.14 lens isn't enough - we need at least augeas 1.13 which can work with no alphanumerics in keys. The tests with 1.11 and 1.14 lens do not die, but also completely ignore kets with a slash

I looked at the puppet-dev-tools image and its using buster to get ruby 2.7.4, but augeas 1.14 does not appear in buster, only next release bookworm. But ruby 2.x is not available in bookworm so we're at a testing impasse 😞 - need augeas 1.14 to test this PR fully

So, yeah, I could go down the patch of creating a bookwork puppet-dev-tools but I am not sure that's the image you are using 🤷

@johnwarburton
Copy link
Author

Hi @bastelfreak

I have dug in and tried to write tests for this particular PR, but have come to the conclusion, that like the PR, all the tests need to be rewritten to cater for a / in a key. My reasoning below that augeas is OK, but the test harness is not

I am really in the weeds here and really do not know what I am doing nor how to progress.

If we start with a vanilla copy of this PR and add a single change - a valid sysctl.conf line that is for configuring VLANs, just the basic tests cannot parse the file correctly

## NB latest version of augeasproviders_core
root@060760e3f1f9:/repo# grep version spec/fixtures/modules/augeasproviders_core/metadata.json
  "version": "4.0.2-rc0",

root@060760e3f1f9:/repo# tail -1 /repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full
net.ipv4.conf.eth0/123.rp_filter = 321

root@060760e3f1f9:/repo# rake spec

Failures:

  1) Puppet::Type::Sysctl::ProviderAugeas with full file should create new entry next to commented out entry
     Failure/Error: raise AugeasSpec::Error, "augparse failed:\n#{output}" unless $CHILD_STATUS == 0 && output.empty?

     AugeasSpec::Error:
       augparse failed:
       Syntax error in lens definition
       Failed to load /tmp/d20230718-3445-exhbz6/test_augeasproviders.aug
       Test failure:/tmp/d20230718-3445-exhbz6/test_augeasproviders.aug:2.2-7.55:
        Expected:
       {
         { "net.bridge.bridge-nf-call-ip6tables" = "0" }
         { "#comment" = "net.bridge.bridge-nf-call-iptables = 0" }
         { "net.bridge.bridge-nf-call-iptables" = "1" }
         { "net.bridge.bridge-nf-call-arptables" = "0" }
       }

        Actual:
       {
         { "net.bridge.bridge-nf-call-ip6tables" = "0" }
         { "#comment" = "net.bridge.bridge-nf-call-iptables = 0" }
         { "net.bridge.bridge-nf-call-iptables" = "1" }
         { "net.bridge.bridge-nf-call-arptables" = "0" }
         { "123.rp_filter" = "321" }
       }

And we can see in the "Actual" results that the tests have parsed and split the net.ipv4.conf.eth0/123.rp_filter key on the slash

Now, I know that augeas isn't the problem as I can use augtool to read in the file and find the VLAN key correctly in the internal augeas format with the slash escaped

root@060760e3f1f9:/repo# augtool --version
augtool 1.14.0 <http://augeas.net/>

root@060760e3f1f9:/repo# augtool --root /repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/ --transform 'Sysctl incl /full'
augtool> print /files/
/files
/files/full
/files/full/#comment[1] = "Kernel sysctl configuration file"
/files/full/#comment[2] = "For binary values, 0 is disabled, 1 is enabled.  See sysctl(8) and"
/files/full/#comment[3] = "sysctl.conf(5) for more details."
/files/full/#comment[4] = "Controls IP packet forwarding"
/files/full/net.ipv4.ip_forward = "0"
/files/full/#comment[5] = "Controls source route verification"
/files/full/net.ipv4.conf.default.rp_filter = "1"
/files/full/#comment[6] = "net.ipv4.conf.default.accept_source_route: Do not accept source routing"
/files/full/net.ipv4.conf.default.accept_source_route = "0"
/files/full/#comment[7] = "SysRq setting"
/files/full/#comment[8] = "kernel.sysrq: controls the System Request debugging functionality of the kernel"
/files/full/kernel.sysrq = "0"
/files/full/#comment[9] = "Controls whether core dumps will append the PID to the core filename."
/files/full/#comment[10] = "Useful for debugging multi-threaded applications."
/files/full/kernel.core_uses_pid = "1"
/files/full/#comment[11] = "Disable netfilter on bridges."
/files/full/net.bridge.bridge-nf-call-ip6tables = "0"
/files/full/#comment[12] = "net.bridge.bridge-nf-call-iptables = 0"
/files/full/net.bridge.bridge-nf-call-arptables = "0"
/files/full/net.ipv4.conf.eth0\/123.rp_filter = "321"

Also its not the augeas ruby bindings - I munged a file from augeas core to simply read in and write out internal representation of the file spec/fixtures/modules/augeasproviders_core/spec/lib/augeas_spec/fixtures.rb

root@060760e3f1f9:/repo# cat fixtures.rb
#!/usr/local/bin/ruby

require 'augeas'

  # Open Augeas on a given file.  Used for testing the results of running
  # Puppet providers.
  def aug_open(file, lens)
    aug = Augeas.open(nil, nil, Augeas::NO_MODL_AUTOLOAD)
    begin
      aug.transform(
        lens: lens,
        name: lens.split('.')[0],
        incl: file,
        excl: []
      )
      aug.set('/augeas/context', "/files#{file}")
      aug.load!
      raise AugeasSpec::Error, "Augeas didn't load #{file}" if aug.match('.').empty?

      yield aug
    rescue Augeas::Error
      errors = []
      aug.match('/augeas//error').each do |errnode|
        aug.match("#{errnode}/*").each do |subnode|
          subvalue = aug.get(subnode)
          errors << "#{subnode} = #{subvalue}"
        end
      end
      raise AugeasSpec::Error, errors.join("\n")
    ensure
      aug.close
    end
  end

target = '/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full'

aug_open(target, "Sysctl.lns") do |aug|
  aug.match("/files/#{target}/*").each do |node|
    val = aug.get(node)
    puts node + ' = ' + val
  end
end

root@060760e3f1f9:/repo# ./fixtures.rb
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[1] = Kernel sysctl configuration file
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[2] = For binary values, 0 is disabled, 1 is enabled.  See sysctl(8) and
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[3] = sysctl.conf(5) for more details.
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[4] = Controls IP packet forwarding
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/net.ipv4.ip_forward = 0
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[5] = Controls source route verification
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/net.ipv4.conf.default.rp_filter = 1
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[6] = net.ipv4.conf.default.accept_source_route: Do not accept source routing
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/net.ipv4.conf.default.accept_source_route = 0
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[7] = SysRq setting
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[8] = kernel.sysrq: controls the System Request debugging functionality of the kernel
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/kernel.sysrq = 0
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[9] = Controls whether core dumps will append the PID to the core filename.
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[10] = Useful for debugging multi-threaded applications.
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/kernel.core_uses_pid = 1
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[11] = Disable netfilter on bridges.
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/net.bridge.bridge-nf-call-ip6tables = 0
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/#comment[12] = net.bridge.bridge-nf-call-iptables = 0
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/net.bridge.bridge-nf-call-arptables = 0
/files/repo/spec/fixtures/unit/puppet/provider/sysctl/augeas/full/net.ipv4.conf.eth0\/123.rp_filter = 321

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Please allow for the use of / as a key separator, as described in the sysctl man page
2 participants
@johnwarburton @bastelfreak